blurb: get BPO# from branch, add commit template #184
Conversation
blurb now gets the branch name from git and auto-fills the bpo number if
the branch name looks like "bpo-1234-whatever". "bpo1234" or
"bpo_1234_whatever" are also supported.
Further more blurb now writes .git/blurb commit template. The first line
of the template is "bpo-{bpo}: suffix of branch name". The body of the
template is the blurb text.
$ git config commit.template .git/blurb
$ git checkout -b bpo-1234-confuse-a-cat
$ blurb
git ci
---
bpo-1234: confuse a cat
blurb text
---
Signed-off-by: Christian Heimes <[email protected]>
|
I like this feature, but for it to be effective, people need to actually create their branch as "bpo-NNNN". |
| sys.exit("Can't find BPO line to ensure there's a space on the end!") | ||
| text = text.replace(without_space, with_space) | ||
| f.write(text) | ||
| # see of the branch name indicated BPO number |
There was a problem hiding this comment.
I realize English is not your first language; may I propose "see if the branch name indicates the BPO number"?
There was a problem hiding this comment.
Our current PEP 8 standard for stdlib comments is to make them complete sentences, with initial cap and period, as in
# See if the branch name indicates the BPO number.
Many old comments do not meet this standard, but new or edited ones should
-- unless Larry as blurb author disagrees ;-).
| text = text.replace(without_space, with_space) | ||
| f.write(text) | ||
| # see of the branch name indicated BPO number | ||
| # makes the 'editor strips trailing whitespace from template lines' |
There was a problem hiding this comment.
Given that your patch obviates (and subsequently removes) the hack in question, this two-line comment is meaningless.
| @@ -888,6 +878,18 @@ def init_tmp_with_template(): | |||
| path = blurb.save_next() | |||
| git_add_files.append(path) | |||
| flush_git_add_files() | |||
|
|
|||
| # For convenience, write a preformatted commit template. Enable template | |||
There was a problem hiding this comment.
Again, a minor suggestion regarding your English here. The first sentence is fine, for the second may I suggest "You can tell git to use this template with "git config commit.template .git/blurb"."
It's not a big deal, but I don't know why you use `` to quote the command here. Simple double-quotes are fine, and if you want something clearer I would indent and put a % to indicate a command prompt.
Also, based on the code, it looks like
- this only works if the ~/.git/blurb directory exists, and
- blurb doesn't create the directory for you automatically.
Does git create the directory for you? If not, we should probably suggest that too in the comment, or else people will follow the instructions only to discover that they don't work.
| msgfile = os.path.join(root, '.git', 'blurb') | ||
| if os.path.isdir(os.path.dirname(msgfile)): | ||
| metadata, text = blurb[0] | ||
| title = branch_suffix.replace('-', ' ').replace('_', ' ') |
There was a problem hiding this comment.
I don't like this. You are replacing all dashes and all underscores with spaces here. This means that a branch called "bpo-1234-Fix-__dir__-on-dict" would be unhelpfully converted into the template " bpo-1234: Fix dir on dict" with three spaces on either side of dict. (GitHub apparently refuses to display the three spaces,
and collapses them to one during formatting.)
If we honor the name of the branch, we should honor the name of the branch. There are legitimate uses for underscores and dashes, and stripping them all out is more surprising than simply leaving them there. I realize git doesn't permit spaces in branch names, so we can't simply write a wonderful commit message in the branch name and have it emerge from the other end of the process perfectly. Ultimately, leaving the dashes and underscores there may be a little inelegant but that's better than outright corrupted.
Please remove this line and resubmit the PR.
| return '', '' | ||
| branch = p.stdout.decode('utf-8').strip() | ||
| mo = re.match("^bpo[-_]?(\d+)[-_]?(.*)", branch) | ||
| if mo is not None: |
There was a problem hiding this comment.
Eschew The Unnecessary Else!
I propose you rewrite these four lines as:
if mo is None:
return '', branch
return mo.group(1), mo.group(2)
Mariatta
added
the
status-stale
|
@tiran I am interested in this feature, do you need help for this code? Thank you |
Mariatta
removed
the
status-stale
|
Closing this in favour of the updated #308. |
blurb now gets the branch name from git and auto-fills the bpo number if
the branch name looks like "bpo-1234-whatever". "bpo1234" or
"bpo_1234_whatever" are also supported.
Further more blurb now writes .git/blurb commit template. The first line
of the template is "bpo-{bpo}: suffix of branch name". The body of the
template is the blurb text.
Signed-off-by: Christian Heimes [email protected]